test(web): add e2e test example to visualizer #1140
Conversation
ZTongci
requested review from
keiya01,
airslice and
mkumbobeaty
as code owners
WalkthroughThe changes introduce enhancements to the end-to-end testing framework for a web application. Updates include modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AuthSetup
participant Browser
participant Dashboard
participant ProjectsPage
participant MapPage
User->>AuthSetup: Validate credentials
AuthSetup->>Browser: Launch Chromium
Browser->>AuthSetup: Navigate to base URL
AuthSetup->>Browser: Fill login form
Browser->>AuthSetup: Submit form
AuthSetup->>Browser: Wait for page load
Browser->>AuthSetup: Verify "New Project" button
AuthSetup->>Browser: Save storage state
Browser->>User: Close browser
Tip OpenAI O1 model for chat
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
✅ Deploy Preview for reearth-web canceled.
|
ZTongci
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (2)
web/playwright.config.ts (1)
11-12: LGTM, but consider running tests in headless mode for faster execution.The changes to the
useobject are approved.
- Setting
headlesstofalsecan be useful for debugging but may slow down test execution. Consider running tests in headless mode for faster execution and only enabling the visible browser window when needed for debugging.- Setting
videoto"retain-on-failure"is a good practice to save storage space.web/e2e/auth.setup.ts (1)
21-33: Consider improving the wait strategy after login.The browser launch and login process are correctly implemented. However, the 10-second wait after logging in might be unnecessary or too long.
Instead of waiting for a fixed duration, consider waiting for a specific condition that indicates the successful login, such as the appearance of a particular element on the page. For example:
-await page.waitForTimeout(10 * 1000); +await expect(page.getByRole("button", { name: "New Project" })).toBeVisible();This way, the test will proceed as soon as the expected element is visible, making the test more efficient and reliable.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
web/e2e/README.mdis excluded by!**/*.md
Files selected for processing (13)
- web/e2e/auth.setup.ts (1 hunks)
- web/e2e/dashboard.spec.ts (1 hunks)
- web/e2e/example.spec.ts (1 hunks)
- web/e2e/pom/index.ts (1 hunks)
- web/e2e/pom/visualizer/dashboard/ProjectsPage.ts (1 hunks)
- web/e2e/pom/visualizer/dashboard/index.ts (1 hunks)
- web/e2e/pom/visualizer/index.ts (1 hunks)
- web/e2e/pom/visualizer/projectSetting/generalPage.ts (1 hunks)
- web/e2e/pom/visualizer/projectSetting/index.ts (1 hunks)
- web/e2e/pom/visualizer/scene/MapPage.ts (1 hunks)
- web/e2e/pom/visualizer/scene/index.ts (1 hunks)
- web/e2e/utils/index.ts (1 hunks)
- web/playwright.config.ts (1 hunks)
Files skipped from review due to trivial changes (6)
- web/e2e/dashboard.spec.ts
- web/e2e/pom/index.ts
- web/e2e/pom/visualizer/dashboard/index.ts
- web/e2e/pom/visualizer/index.ts
- web/e2e/pom/visualizer/projectSetting/index.ts
- web/e2e/pom/visualizer/scene/index.ts
Additional comments not posted (24)
web/e2e/pom/visualizer/projectSetting/generalPage.ts (2)
3-28: LGTM!The
generalPageclass is well-structured and follows the Page Object Model (POM) pattern for organizing the interactions with the general project settings page. The class encapsulates the page-specific elements and actions, making the tests more readable and maintainable.
7-15: LGTM!The constructor correctly initializes the
pageproperty and locates the project name element using a specific selector. The locator is resilient to changes in the page structure, making the tests more maintainable.web/playwright.config.ts (3)
1-1: LGTM!The change to the import statement is approved.
18-32: LGTM!The addition of the
projectsproperty is approved.
- Defining multiple project configurations can be useful for running tests across different environments or devices.
- The configuration for "chromium" looks good and includes reasonable options for running tests.
15-16: LGTM, but verify the implementation of the new setup file.The change to the
globalSetupproperty is approved.However, ensure that the new setup file
./e2e/auth.setup.tsis implemented correctly and provides the necessary setup for the tests.Run the following script to verify the implementation of the new setup file:
web/e2e/pom/visualizer/dashboard/ProjectsPage.ts (5)
9-14: LGTM!The constructor is correctly initializing the
ProjectsPageclass with the requiredPageobject and locators.
16-18: LGTM!The
gotomethod is correctly navigating to the root URL.
20-24: LGTM!The
createProjectmethod is correctly creating a new project by interacting with the new project button, project name input, and apply button.
26-30: LGTM!The
dbClickInProjectByNamemethod is correctly double-clicking on a project with the given name.
32-38: LGTM!The
intoProjectSettingByNamemethod is correctly navigating to the project settings page for a project with the given name.web/e2e/auth.setup.ts (2)
1-19: LGTM!The imports and environment variable handling look good:
- The necessary dependencies are imported correctly.
- Reading the account credentials from environment variables is a secure approach.
- Throwing an error when the required variables are missing is a good practice to prevent unexpected behavior.
34-40: LGTM!Storing the authenticated session state is correctly implemented:
- Saving the session state to a file allows reusing the session across multiple tests.
- Using
__dirnameto construct the file path ensures that the file is saved in the correct location.- The
.jsonfile extension indicates that the session state is saved in a standard and interoperable format.web/e2e/pom/visualizer/scene/MapPage.ts (6)
12-24: LGTM!The constructor is well-structured and initializes the locators for various elements on the page using appropriate methods based on the element's attributes. The locators are stored as instance variables, making them accessible to other methods in the class.
26-28: LGTM!The
gotomethod is simple and straightforward, navigating to the root URL of the application.
30-36: LGTM!The
createNewLayerBySketchmethod automates the process of creating a new layer by sketch. It performs the necessary actions in the correct order and waits for the "New Layer" button to be visible before proceeding, ensuring that the page has loaded properly. The method takes the layer name as a parameter, making it reusable for creating different layers.
38-44: LGTM!The
getLocatorOfCanvsmethod is useful for retrieving the coordinates of the center of the canvas, which can be used for performing actions on the canvas, such as creating a circle. The method throws an error if no canvas is found, providing a clear indication of the issue.
46-52: LGTM!The
createCircleToEarthByLocatormethod automates the process of creating a circle on the earth using the specified coordinates. The method performs the necessary actions in the correct order, including waiting for 5 seconds after the first click to allow the earth to load properly. The method also adds a delay of 500 milliseconds to the second click to ensure that the circle is created properly.
54-57: LGTM!The
closeSkyBoxmethod automates the process of closing the sky box. It performs the necessary actions in the correct order, first clicking on the "Sky" text to open the sky box options and then clicking on the sky box select button to close the sky box.web/e2e/example.spec.ts (4)
1-10: LGTM!The imports and constants are correctly defined.
12-16: LGTM!The session setup code is correctly implemented.
45-59: LGTM!The visual regression testing code is correctly implemented.
61-67: LGTM!The project deletion code is correctly implemented.
web/e2e/utils/index.ts (2)
Line range hint
1-10: LGTM!The changes to import
chromiumandLocatorfrom Playwright are approved.Importing these additional types and the browser instance enhances the testing capabilities by providing access to useful Playwright APIs.
Disabling ESLint for
chromiumimport is acceptable as it may be re-exported for use in other files.
16-16: LGTM!The changes to re-export
expect,chromium,Page, andLocatorfrom Playwright are approved.Re-exporting these types and functions allows them to be imported from this file in other parts of the codebase, which enhances the usability and maintainability of the testing setup.
airslice
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
web/e2e/editorMapAddLayer.spec.ts (1)
18-68: Great test structure! Consider improving the reliability and configurability of the screenshot comparison.The test case follows a clear structure and uses a Page Object Model (POM) for interacting with the application, which is a good practice for maintainability and readability. The use of unique identifiers for the project and layer names, waiting for the canvas to stabilize before taking the screenshot, deleting the created project at the end, using environment variables for the base URL, and using a session stored in a JSON file for authentication are all good practices.
However, there are a few areas that could be improved:
The test case uses a hardcoded timeout of 5 seconds for waiting for the canvas to stabilize. This can be brittle and may need to be adjusted based on the performance of the application. Consider using a more reliable way to wait for the canvas to stabilize, such as waiting for a specific element to be visible or for the network requests to complete.
The test case uses a hardcoded value of 10 for the maximum number of different pixels allowed in the screenshot comparison. This may need to be adjusted based on the expected level of variance in the rendering. Consider making this value configurable, either through an environment variable or a configuration file.
The test case uses a soft assertion for comparing the screenshot with the expected image, which allows the test to continue even if the assertion fails. While this can be useful for debugging, it's important to ensure that the test fails if any assertion fails. Consider adding a check after the soft assertion to fail the test if the assertion failed.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
web/e2e/editorMapAddLayer.spec.ts-snapshots/editorMapAddLayerExpect-chromium-darwin.pngis excluded by!**/*.png
Files selected for processing (9)
- web/.gitignore (1 hunks)
- web/e2e/dashboard.spec.ts (1 hunks)
- web/e2e/editorMapAddLayer.spec.ts (1 hunks)
- web/e2e/pom/visualizer/editor/MapPage.ts (1 hunks)
- web/e2e/pom/visualizer/editor/index.ts (1 hunks)
- web/e2e/pom/visualizer/index.ts (1 hunks)
- web/e2e/utils/config.ts (1 hunks)
- web/e2e/utils/login.ts (1 hunks)
- web/e2e/utils/setup.ts (1 hunks)
Files skipped from review due to trivial changes (6)
- web/.gitignore
- web/e2e/dashboard.spec.ts
- web/e2e/pom/visualizer/editor/index.ts
- web/e2e/utils/config.ts
- web/e2e/utils/login.ts
- web/e2e/utils/setup.ts
Files skipped from review as they are similar to previous changes (1)
- web/e2e/pom/visualizer/index.ts
Additional comments not posted (1)
web/e2e/pom/visualizer/editor/MapPage.ts (1)
1-58: LGTM!The
MapPageclass follows the Page Object Model (POM) pattern and uses Playwright's recommended approaches for interacting with the page. The class contains methods for common actions in the visualizer, such as creating a new layer, creating a circle on the earth, and closing the sky box. The class uses appropriate selectors, waiting mechanisms, mouse actions, error handling, and naming conventions, making the code readable and maintainable.
ZTongci
force-pushed
the
feat/e2e_test
branch
from
04701d2
to
4042d4b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
web/playwright.config.ts (2)
12-12: Ensure tests are run in both headless and non-headless modes.Running tests in a visible browser window by setting
headlesstofalsecan be useful for debugging purposes. However, it's important to ensure that tests are also run in headless mode, especially in CI/CD pipelines, to catch any issues that may occur specifically in headless mode.Consider adding a configuration option to control the
headlesssetting based on the environment (e.g.,process.env.CI) to automatically run tests in headless mode in CI/CD pipelines.
29-29: Use--no-sandboxwith caution.The
launchOptions.argsproperty includes the--no-sandboxargument, which disables the sandboxing feature. While this may be necessary for certain environments, it's important to be aware of the potential security implications.Disabling the sandbox may make the browser more vulnerable to security threats. Consider enabling the sandbox whenever possible and only disable it when absolutely necessary. If the sandbox must be disabled, ensure that appropriate security measures are in place to mitigate any risks.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
web/e2e/README.mdis excluded by!**/*.md
Files selected for processing (18)
- web/.env.example (1 hunks)
- web/.gitignore (1 hunks)
- web/e2e/auth.setup.ts (1 hunks)
- web/e2e/dashboard.spec.ts (1 hunks)
- web/e2e/editorMapAddLayer.spec.ts (1 hunks)
- web/e2e/pom/index.ts (1 hunks)
- web/e2e/pom/visualizer/dashboard/ProjectsPage.ts (1 hunks)
- web/e2e/pom/visualizer/dashboard/index.ts (1 hunks)
- web/e2e/pom/visualizer/editor/MapPage.ts (1 hunks)
- web/e2e/pom/visualizer/editor/index.ts (1 hunks)
- web/e2e/pom/visualizer/index.ts (1 hunks)
- web/e2e/pom/visualizer/projectSetting/generalPage.ts (1 hunks)
- web/e2e/pom/visualizer/projectSetting/index.ts (1 hunks)
- web/e2e/utils/config.ts (1 hunks)
- web/e2e/utils/index.ts (1 hunks)
- web/e2e/utils/login.ts (1 hunks)
- web/e2e/utils/setup.ts (1 hunks)
- web/playwright.config.ts (1 hunks)
Files skipped from review due to trivial changes (7)
- web/e2e/pom/index.ts
- web/e2e/pom/visualizer/dashboard/index.ts
- web/e2e/pom/visualizer/editor/index.ts
- web/e2e/pom/visualizer/projectSetting/index.ts
- web/e2e/utils/config.ts
- web/e2e/utils/login.ts
- web/e2e/utils/setup.ts
Files skipped from review as they are similar to previous changes (10)
- web/.env.example
- web/.gitignore
- web/e2e/auth.setup.ts
- web/e2e/dashboard.spec.ts
- web/e2e/editorMapAddLayer.spec.ts
- web/e2e/pom/visualizer/dashboard/ProjectsPage.ts
- web/e2e/pom/visualizer/editor/MapPage.ts
- web/e2e/pom/visualizer/index.ts
- web/e2e/pom/visualizer/projectSetting/generalPage.ts
- web/e2e/utils/index.ts
Additional comments not posted (6)
web/playwright.config.ts (6)
1-1: LGTM!The import statement has been correctly updated to include
devicesfrom the Playwright library, which will enhance the configuration's capability to specify device settings for tests.
15-15: LGTM!The
snapshotPathTemplateproperty has been correctly added to specify the path template for storing snapshot files.
16-17: Verify the correctness of the new setup file.The
globalSetupproperty has been modified to point to a new setup file,./e2e/auth.setup.ts, replacing the previous setup file. This change may reflect a shift in how the test environment is initialized, potentially indicating a more streamlined or updated setup process.Ensure that the new setup file is correctly implemented and does not introduce any issues. Verify that the setup process works as expected and all necessary dependencies are properly initialized.
18-18: LGTM!The
reporterproperty has been correctly modified to conditionally use the "github" reporter in CI environments and the "list" reporter otherwise, which improves the reporting of test results based on the environment.
19-33: LGTM!The new
projectsarray has been correctly added, defining a project configuration for "chromium". The configuration utilizes thedevices["Desktop Chrome"]settings and includes specific options for running tests, such as enabling screenshots and video recording during test execution.The
launchOptionshave been adjusted to include arguments that disable the sandboxing feature, which may be necessary for certain environments.Overall, these changes enhance the configuration's flexibility and usability, particularly for running tests across different environments.
28-29: Verify the impact of removing the commented-out launch arguments.The
launchOptions.argsproperty includes a commented-out line with additional launch arguments:// args: ["--headless","--no-sandbox","--use-angle=gl"]It's important to ensure that removing the
--headlessand--use-angle=glarguments does not introduce any issues or affect the test execution.Verify that the tests still run correctly without these arguments and that there are no unintended side effects. If necessary, consider adding comments to explain why these arguments were removed.
Overview
add e2e test example to visualizer
What I've done
create session once and use this session to login and testing.
add pom structure for e2e test.
add e2e test example to visualizer that create layer and add circle into earth.
What I haven't done
I don't change old e2e test structure. And didn't use auth0 login function.
How I tested
Which point I want you to review particularly
Memo
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
.gitignoreto exclude sensitive files and directories from version control.